Migrate ERC7786Recipient from community #5904
Conversation
🦋 Changeset detectedLatest commit: a669514 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Reviewer's GuideMigrate the community ERC-7786 receiver implementation into core by updating the interface, adding a gateway mock, defining an abstract receiver contract, providing a receiver mock, and introducing accompanying tests and a changeset. Sequence diagram for cross-chain message delivery using ERC7786GatewayMock and ERC7786ReceiverMocksequenceDiagram
participant Sender
participant ERC7786GatewayMock
participant ERC7786ReceiverMock
Sender->>ERC7786GatewayMock: sendMessage(recipient, payload, attributes)
ERC7786GatewayMock->>ERC7786ReceiverMock: receiveMessage(receiveId, sender, payload)
ERC7786ReceiverMock->>ERC7786ReceiverMock: _isKnownGateway(msg.sender)
ERC7786ReceiverMock->>ERC7786ReceiverMock: _processMessage(gateway, receiveId, sender, payload)
ERC7786ReceiverMock-->>ERC7786GatewayMock: return magic value
ERC7786GatewayMock-->>Sender: return sendId
Entity relationship diagram for MessageSent event attributeserDiagram
MESSAGE_SENT {
bytes32 sendId
bytes sender
bytes recipient
bytes payload
uint256 value
bytes[] attributes
}
MESSAGE_SENT ||--o| ATTRIBUTES : contains
ATTRIBUTES {
bytes attribute
}
Class diagram for ERC7786Receiver and related contractsclassDiagram
class IERC7786GatewaySource {
<<interface>>
+supportsAttribute(bytes4 selector) bool
+sendMessage(bytes recipient, bytes payload, bytes[] attributes) bytes32
<<event>> MessageSent
}
class IERC7786Receiver {
<<interface>>
+receiveMessage(bytes32 receiveId, bytes sender, bytes payload) bytes4
}
class ERC7786GatewayMock {
<<abstract>>
-_lastReceiveId uint256
+supportsAttribute(bytes4 selector) bool
+sendMessage(bytes recipient, bytes payload, bytes[] attributes) bytes32
}
class ERC7786Receiver {
<<abstract>>
+receiveMessage(bytes32 receiveId, bytes sender, bytes payload) bytes4
-_isKnownGateway(address instance) bool
-_processMessage(address gateway, bytes32 receiveId, bytes sender, bytes payload)
}
class ERC7786ReceiverMock {
-_gateway address
+constructor(address gateway_)
+_isKnownGateway(address instance) bool
+_processMessage(address gateway, bytes32 receiveId, bytes sender, bytes payload)
<<event>> MessageReceived
}
IERC7786GatewaySource <|.. ERC7786GatewayMock
IERC7786Receiver <|.. ERC7786Receiver
ERC7786Receiver <|-- ERC7786ReceiverMock
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedUse the following commands to manage reviews:
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds ERC-7786 recipient base and mocks, renames Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant G as Gateway (ERC7786GatewayMock)
participant R as Recipient (ERC7786Recipient)
participant C as Concrete Recipient (ERC7786RecipientMock)
G->>R: call receiveMessage(receiveId, sender, payload) [+value]
Note right of R: msg.sender == gateway
alt gateway authorized
R->>R: check per-gateway BitMap(receiveId)
R->>C: _processMessage(gateway, receiveId, sender, payload)
C-->>R: emit MessageReceived
R-->>G: return selector (IERC7786Recipient.receiveMessage.selector)
Note left of G: verify selector → emit MessageSent
else unauthorized
R-->>G: revert ERC7786RecipientUnauthorizedGateway(gateway, sender)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches🧪 Generate unit tests
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `contracts/mocks/crosschain/ERC7786GatewayMock.sol:54` </location>
<code_context>
+ attributes
+ );
+
+ return 0;
+ }
+}
</code_context>
<issue_to_address>
Returning a constant zero for sendId may limit traceability in tests.
Using a fixed sendId may hinder identifying individual messages during testing. Suggest returning a unique or incremented value for better traceability.
Suggested implementation:
```
emit MessageSent(
bytes32(_sendIdCounter + 1),
InteroperableAddress.formatEvmV1(block.chainid, msg.sender),
recipient,
payload,
msg.value,
attributes
);
```
```
_sendIdCounter += 1;
return bytes32(_sendIdCounter);
}
}
```
```
// Counter for unique sendId values
uint256 private _sendIdCounter;
// emit standard event
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
.changeset/silent-zebras-press.md (1)
5-5: Hyphenate “cross‑chain”.Minor wording polish for consistency with the rest of the repo.
-`ERC7786Receiver`: Boilerplate contract for receiving ERC-7786 crosschain messages. +`ERC7786Receiver`: Boilerplate contract for receiving ERC-7786 cross-chain messages.contracts/interfaces/draft-IERC7786.sol (1)
12-14: Docs still mentionoutboxId; update tosendId.Comment blocks reference
outboxIdwhile the event/return usesendId. Align to prevent confusion.- * @dev Event emitted when a message is created. If `outboxId` is zero, no further processing is necessary. If - * `outboxId` is not zero, then further (gateway specific, and non-standardized) action is required. + * @dev Event emitted when a message is created. If `sendId` is zero, no further processing is necessary. If + * `sendId` is not zero, then further (gateway specific, and non-standardized) action is required. @@ - * @dev Endpoint for creating a new message. If the message requires further (gateway specific) processing before - * it can be sent to the destination chain, then a non-zero `outboxId` must be returned. Otherwise, the - * message MUST be sent and this function must return 0. + * @dev Endpoint for creating a new message. If the message requires further (gateway specific) processing before + * it can be sent to the destination chain, then a non-zero `sendId` must be returned. Otherwise, the message + * MUST be sent and this function must return 0x00...00.Also applies to: 31-35
contracts/crosschain/ERC7786Receiver.sol (1)
23-27: Optional: make the functionexternalto match the interface and save gas.Matches
IERC7786Receiverand avoids an extra ABI-to-memory copy.- ) public payable virtual returns (bytes4) { + ) external payable virtual returns (bytes4) {contracts/mocks/crosschain/ERC7786GatewayMock.sol (4)
45-52: Confirm event parameter semantics fordestination.You emit
destinationasbytes32(0). If the interface expects a concrete destination identifier even on same-chain mocks, consider using a deterministic value (e.g.,bytes32(block.chainid)) or document that zero is intentional.Would you like me to adjust the event emission and tests accordingly?
14-15: Counter type choice.
_lastReceiveIdcan be narrower (e.g.,uint64) for cheaper storage if you want to signal mock-only intent. Not required.
21-55: Optional: return a meaningfulsendIdfor better testability.Currently always returns zero. Returning the receive id improves correlation in tests.
- ) public payable virtual override returns (bytes32 sendId) { + ) public payable virtual override returns (bytes32 sendId) { // attributes are not supported if (attributes.length > 0) { revert IERC7786GatewaySource.UnsupportedAttribute(bytes4(attributes[0])); } // parse recipient (bool success, uint256 chainid, address target) = recipient.tryParseEvmV1Calldata(); if (!(success && chainid == block.chainid)) { revert InvalidDestination(); } // perform call - bytes4 magic = IERC7786Receiver(target).receiveMessage{value: msg.value}( - bytes32(++_lastReceiveId), - InteroperableAddress.formatEvmV1(block.chainid, msg.sender), - payload - ); + bytes32 id = bytes32(++_lastReceiveId); + bytes memory sender = InteroperableAddress.formatEvmV1(block.chainid, msg.sender); + bytes4 magic = IERC7786Receiver(target).receiveMessage{value: msg.value}(id, sender, payload); if (magic != IERC7786Receiver.receiveMessage.selector) { revert ReceiverError(); } // emit standard event emit MessageSent( - bytes32(0), - InteroperableAddress.formatEvmV1(block.chainid, msg.sender), + bytes32(0), + sender, recipient, payload, msg.value, attributes ); - return bytes32(0); + return id; // or keep bytes32(0) if intentional }If tests rely on zero, keep the current behavior and add a brief note in NatSpec.
16-19: Minor: state mutability.Interface says
view, so keep asview. If interface ever changes topure, this function can be tightened.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/silent-zebras-press.md(1 hunks)contracts/crosschain/ERC7786Receiver.sol(1 hunks)contracts/interfaces/draft-IERC7786.sol(1 hunks)contracts/mocks/crosschain/ERC7786GatewayMock.sol(1 hunks)contracts/mocks/crosschain/ERC7786ReceiverMock.sol(1 hunks)test/crosschain/ERC7786Receiver.test.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/crosschain/ERC7786Receiver.test.js (1)
test/helpers/random.js (1)
generators(3-10)
🪛 LanguageTool
.changeset/silent-zebras-press.md
[grammar] ~5-~5: There might be a mistake here.
Context: ... receiving ERC-7786 crosschain messages.
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Redirect rules - solidity-contracts
- GitHub Check: tests
- GitHub Check: tests-foundry
- GitHub Check: slither
- GitHub Check: coverage
- GitHub Check: tests-upgradeable
- GitHub Check: halmos
- GitHub Check: Header rules - solidity-contracts
- GitHub Check: Pages changed - solidity-contracts
🔇 Additional comments (5)
.changeset/silent-zebras-press.md (1)
2-3: Confirm semver bump for event parameter rename
- Package name ‘openzeppelin-solidity’ matches package.json, so no update needed.
- Renaming the event parameter (receiver → recipient) doesn’t change the ABI signature but will break generated typings (e.g., TypeChain); confirm whether a minor bump is sufficient or if this deserves a major bump.
contracts/interfaces/draft-IERC7786.sol (1)
18-18: Param rename aligns terminology, but watch downstream typings.Changing
receiver→recipientis good for clarity, but it alters the “name” field in the ABI and may break typed consumers. Ensure the release note calls this out and the changeset version is appropriate.contracts/mocks/crosschain/ERC7786ReceiverMock.sol (1)
7-27: Mock looks correct and minimal.Gateway whitelisting and event emission mirror the base contract’s expectations. Good for tests.
test/crosschain/ERC7786Receiver.test.js (1)
9-9: No changes required forgenerators.hexBytesusage
Verified in test/helpers/random.js (line 20) thatgenerators.hexBytesis aliased togenerators.bytes, so the fallback is unnecessary.contracts/mocks/crosschain/ERC7786GatewayMock.sol (1)
8-13: Good: minimal mock surface and explicit custom errors.Clean separation of concerns; mock keeps observable behavior tight.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
contracts/crosschain/README.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: halmos
- GitHub Check: coverage
Co-authored-by: Ernesto García <[email protected]>
Co-authored-by: Ernesto García <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
contracts/interfaces/draft-IERC7786.sol (2)
12-16: Doc references “outboxId” but the ABI exposes “sendId.”Update wording to avoid confusion.
- * @dev Event emitted when a message is created. If `outboxId` is zero, no further processing is necessary. If - * `outboxId` is not zero, then further (gateway specific, and non-standardized) action is required. + * @dev Event emitted when a message is created. If `sendId` is zero, no further processing is necessary. If + * `sendId` is not zero, then further (gateway specific, and non-standardized) action is required.
31-39: Clarify return semantics in sendMessage docs.Align with the return type and event.
- * @dev Endpoint for creating a new message. If the message requires further (gateway specific) processing before - * it can be sent to the destination chain, then a non-zero `outboxId` must be returned. Otherwise, the - * message MUST be sent and this function must return 0. + * @dev Endpoint for creating a new message. If the message requires further (gateway‑specific) processing before + * it can be sent to the destination chain, a non‑zero `sendId` MUST be returned. Otherwise, the message MUST be + * sent and this function MUST return 0x00..00.
♻️ Duplicate comments (2)
contracts/crosschain/README.adoc (2)
10-12: Replace placeholder with resolvable xref and adjust heading.-== Helpers - -{{IERC7786Recipient}} +== Helper contracts + +- xref:api/crosschain.adoc#IERC7786Recipient[IERC7786Recipient]
6-6: Fix agreement and hyphenation in body text.-This directory contains contracts for sending and receiving cross chain messages that follows the ERC-7786 standard. +This directory contains contracts for sending and receiving cross-chain messages that follow the ERC-7786 standard.
🧹 Nitpick comments (7)
contracts/interfaces/draft-IERC7786.sol (1)
48-52: Use “recipient” consistently in the interface docs.- * @dev Interface for the ERC-7786 client contract (receiver). + * @dev Interface for the ERC-7786 client contract (recipient).contracts/crosschain/README.adoc (1)
8-8: Hyphenate “crosschain” and tighten wording.-- {IERC7786Recipient}: generic ERC-7786 crosschain contract that receives messages from a trusted gateway +- {IERC7786Recipient}: generic ERC-7786 cross-chain contract that receives messages from a trusted gatewaycontracts/mocks/crosschain/ERC7786RecipientMock.sol (2)
10-10: Index key event fields for better test/dev UX.- event MessageReceived(address gateway, bytes32 receiveId, bytes sender, bytes payload, uint256 value); + event MessageReceived(address indexed gateway, bytes32 indexed receiveId, bytes sender, bytes payload, uint256 value);
8-9: Optional: add a getter for_gateway.Improves introspection and debugging without exposing mutability.
function gateway() external view returns (address) { return _gateway; }contracts/crosschain/ERC7786Recipient.sol (3)
33-43: Consider reentrancy posture for _processMessageDepending on concrete implementations,
_processMessagemay transfer value or interact with untrusted contracts. Consider documenting reentrancy expectations here, or guarding in derived contracts (e.g., OZ ReentrancyGuard) and adding tests for reentrancy attempts via the gateway.
13-16: Nit: wording/grammar in NatSpecMinor clarity and US spelling.
- * * {_isKnownGateway}, an internal getter used to verify whether an address is recognised by the contract as a valid - * ERC-7786 destination gateway. One or multiple gateway can be supported. Note that any malicious address for which + * * {_isKnownGateway}, an internal getter used to verify whether an address is recognized by the contract as a valid + * ERC-7786 destination gateway. One or multiple gateways can be supported. Note that any malicious address for which
34-34: Nit: parameter nameRename
instancetogatewayfor consistency with usage elsewhere in this file.- function _isKnownGateway(address instance) internal view virtual returns (bool); + function _isKnownGateway(address gateway) internal view virtual returns (bool);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/silent-zebras-press.md(1 hunks)contracts/crosschain/ERC7786Recipient.sol(1 hunks)contracts/crosschain/README.adoc(1 hunks)contracts/interfaces/draft-IERC7786.sol(2 hunks)contracts/mocks/crosschain/ERC7786GatewayMock.sol(1 hunks)contracts/mocks/crosschain/ERC7786RecipientMock.sol(1 hunks)test/crosschain/ERC7786Recipient.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/mocks/crosschain/ERC7786GatewayMock.sol
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-28T15:48:30.684Z
Learnt from: ernestognw
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/README.adoc:1-1
Timestamp: 2025-08-28T15:48:30.684Z
Learning: ernestognw prefers "Cross chain" without hyphenation rather than "Cross-chain" in documentation titles.
Applied to files:
contracts/crosschain/README.adoc
📚 Learning: 2025-08-28T16:58:18.790Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786GatewayMock.sol:27-31
Timestamp: 2025-08-28T16:58:18.790Z
Learning: In the OpenZeppelin ERC7786 implementation, the `UnsupportedAttribute(bytes4 selector)` error is defined in the `IERC7786GatewaySource` interface, making it directly accessible to inheriting contracts without needing interface qualification.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
🧬 Code graph analysis (1)
test/crosschain/ERC7786Recipient.test.js (1)
test/helpers/random.js (1)
generators(3-10)
🪛 LanguageTool
.changeset/silent-zebras-press.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...ic crosschain message receiver contract.
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: halmos
- GitHub Check: coverage
- GitHub Check: tests-upgradeable
- GitHub Check: tests-foundry
- GitHub Check: tests
🔇 Additional comments (4)
contracts/interfaces/draft-IERC7786.sol (1)
18-22: Event param rename LGTM.“receiver” → “recipient” matches the rest of the PR.
contracts/crosschain/README.adoc (1)
1-1: Title style acknowledged.Keeping “Cross chain” (no hyphen) per documented preference.
test/crosschain/ERC7786Recipient.test.js (1)
35-35: Second arg toethers.toBeHexmust be a number, not BigInt.- .withArgs(this.gateway, ethers.toBeHex(1n, 32n), this.toErc7930(this.sender), payload, value); + .withArgs(this.gateway, ethers.toBeHex(1n, 32), this.toErc7930(this.sender), payload, value);Likely an incorrect or invalid review comment.
contracts/crosschain/ERC7786Recipient.sol (1)
28-28: Fix: invalid use of require with a custom error
requirecannot take a custom error as its second argument. Use an explicit revert.- require(_isKnownGateway(msg.sender), ERC7786RecipientInvalidGateway(msg.sender)); + if (!_isKnownGateway(msg.sender)) { + revert ERC7786RecipientInvalidGateway(msg.sender); + }⛔ Skipped due to learnings
Learnt from: ernestognw PR: OpenZeppelin/openzeppelin-contracts#5904 File: contracts/crosschain/ERC7786Receiver.sol:28-31 Timestamp: 2025-08-28T15:48:27.419Z Learning: Since Solidity 0.8.26, the require() function supports a new overload require(bool, Error) that allows using custom errors as the second parameter instead of just string messages. This feature was initially available only in the via-ir pipeline in 0.8.26, but became available in the legacy pipeline starting from Solidity 0.8.27.Learnt from: Amxx PR: OpenZeppelin/openzeppelin-contracts#5904 File: contracts/mocks/crosschain/ERC7786GatewayMock.sol:27-31 Timestamp: 2025-08-28T16:58:18.790Z Learning: In the OpenZeppelin ERC7786 implementation, the `UnsupportedAttribute(bytes4 selector)` error is defined in the `IERC7786GatewaySource` interface, making it directly accessible to inheriting contracts without needing interface qualification.
Amxx
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
contracts/crosschain/ERC7786Recipient.sol (1)
22-31: Fix: add missingoverrideand use custom error withrevert(compile blockers)The function implements the interface but lacks
override. Also,require(..., ERC7786RecipientInvalidGateway(...))is invalid Solidity; userevertwith the custom error.- /// @inheritdoc IERC7786Recipient - function receiveMessage( + /// @inheritdoc IERC7786Recipient + function receiveMessage( bytes32 receiveId, bytes calldata sender, // Binary Interoperable Address bytes calldata payload - ) external payable returns (bytes4) { - require(_isAuthorizedGateway(msg.sender, sender), ERC7786RecipientInvalidGateway(msg.sender)); + ) external payable override returns (bytes4) { + if (!_isAuthorizedGateway(msg.sender, sender)) { + revert ERC7786RecipientInvalidGateway(msg.sender); + } _processMessage(msg.sender, receiveId, sender, payload); return IERC7786Recipient.receiveMessage.selector; }
🧹 Nitpick comments (3)
contracts/crosschain/ERC7786Recipient.sol (3)
33-35: Gas/consistency: prefercalldataand consistent param namingAvoid copying
senderto memory in the auth hook and align the param name with_processMessage.- /// @dev Virtual getter that returns whether an address is a valid ERC-7786 gateway for a given sender. - function _isAuthorizedGateway(address instance, bytes memory sender) internal view virtual returns (bool); + /// @dev Virtual getter that returns whether an address is a valid ERC-7786 gateway for a given sender. + function _isAuthorizedGateway(address gateway, bytes calldata sender) internal view virtual returns (bool);
37-42: Optional: dropgatewayparam from_processMessage(smaller stack, simpler API)You can read
msg.senderinside overrides to get the gateway. This reduces stack usage and nudges devs to authorize based onmsg.sender.- function _processMessage( - address gateway, - bytes32 receiveId, - bytes calldata sender, - bytes calldata payload - ) internal virtual; + function _processMessage( + bytes32 receiveId, + bytes calldata sender, + bytes calldata payload + ) internal virtual;If you adopt this, update all overrides and the call site:
- _processMessage(msg.sender, receiveId, sender, payload); + _processMessage(receiveId, sender, payload);
7-18: Nit: terminology and grammar in NatSpecAlign terminology with “Recipient” and fix minor grammar.
- * @dev Base implementation of an ERC-7786 compliant cross-chain message receiver. + * @dev Base implementation of an ERC-7786 compliant cross-chain message recipient. @@ - * * {_isAuthorizedGateway}, an internal getter used to verify whether an address is recognised by the contract as a - * valid ERC-7786 destination gateway. One or multiple gateway can be supported. Note that any malicious address for + * * {_isAuthorizedGateway}, an internal getter used to verify whether an address is recognized by the contract as a + * valid ERC-7786 destination gateway. One or multiple gateways can be supported. Note that any malicious address for
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
contracts/crosschain/ERC7786Recipient.sol(1 hunks)contracts/mocks/crosschain/ERC7786RecipientMock.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/mocks/crosschain/ERC7786RecipientMock.sol
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-08-29T13:17:07.068Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:22-31
Timestamp: 2025-08-29T13:17:07.068Z
Learning: In OpenZeppelin contracts, virtual functions that implement interfaces should use `public` visibility instead of `external`, even for calldata-heavy functions. This is because external functions cannot be called with `super` in derived contracts, which would limit extensibility. OpenZeppelin has strong guidelines to keep all public-facing virtual functions as `public` to maintain inheritance flexibility.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests
- GitHub Check: tests-foundry
- GitHub Check: coverage
- GitHub Check: halmos
- GitHub Check: tests-upgradeable
- GitHub Check: slither
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
contracts/crosschain/ERC7786Recipient.sol (3)
31-36: Considerpublic virtual overrideto allowsuperin derived contracts.Per OZ guideline for extensible bases, prefer
publicvisibility for virtual entrypoints so children cansuper.receiveMessage(...). Keepexternalonly if you intentionally want to prevent overrides.- ) external payable override returns (bytes4) { + ) public payable virtual override returns (bytes4) {
53-58: Optional: drop thegatewayparam and readmsg.senderinside_processMessage.This reduces stack usage and matches how the gateway is already sourced. Apply only if you don’t need to pass a different address for testing/mocking.
- function _processMessage( - address gateway, - bytes32 receiveId, - bytes calldata sender, - bytes calldata payload - ) internal virtual; + function _processMessage( + bytes32 receiveId, + bytes calldata sender, + bytes calldata payload + ) internal virtual;
31-36: Add missingoverrideon the interface implementation (compile error).This function implements
IERC7786Recipientbut lacks theoverridespecifier, which is required by Solidity.- ) external payable returns (bytes4) { + ) external payable override returns (bytes4) {
🧹 Nitpick comments (2)
contracts/crosschain/ERC7786Recipient.sol (2)
15-16: Fix NatSpec typos/wording.Minor grammar and typo fixes improve docs clarity (“gateways”, “meaning”, “recognized”).
- * valid ERC-7786 destination gateway. One or multiple gateway can be supported. Note that any malicious address for - * which this function returns true would be able to impersonate any account on any other chain sending any message. + * valid ERC-7786 destination gateway. One or multiple gateways can be supported. Note that any malicious address for + * which this function returns true would be able to impersonate any account on any other chain, sending any message. @@ - * This contract implements replay protection, manning that if two messages are received from the same gateway with the + * This contract implements replay protection, meaning that if two messages are received from the same gateway with theAlso applies to: 20-22
49-51: Rename paraminstance→gatewayfor consistency.Elsewhere you use “gateway”; aligning names improves readability.
- function _isAuthorizedGateway(address instance, bytes calldata sender) internal view virtual returns (bool); + function _isAuthorizedGateway(address gateway, bytes calldata sender) internal view virtual returns (bool);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
contracts/crosschain/ERC7786Recipient.sol(1 hunks)contracts/mocks/crosschain/ERC7786RecipientMock.sol(1 hunks)test/crosschain/ERC7786Recipient.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/mocks/crosschain/ERC7786RecipientMock.sol
- test/crosschain/ERC7786Recipient.test.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-08-29T13:17:07.068Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:22-31
Timestamp: 2025-08-29T13:17:07.068Z
Learning: In OpenZeppelin contracts, virtual functions that implement interfaces should use `public` visibility instead of `external`, even for calldata-heavy functions. This is because external functions cannot be called with `super` in derived contracts, which would limit extensibility. OpenZeppelin has strong guidelines to keep all public-facing virtual functions as `public` to maintain inheritance flexibility.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: coverage
- GitHub Check: halmos
- GitHub Check: slither
- GitHub Check: tests
🔇 Additional comments (3)
contracts/crosschain/ERC7786Recipient.sol (3)
42-46: Replay-protection order is correct.Marking the
receiveIdas processed before delegating to_processMessageprevents reentrant double-processing and will roll back on revert. LGTM.
26-30: Errors are well-scoped and informative.Clear, specific custom errors for invalid gateway and replay. Nice.
37-41: Use custom errors withrevert, not asrequiremessages.
requireonly accepts a boolean and a string message. Passing a custom error torequirewon’t compile. Useif (!cond) revert Error(...).- require(_isAuthorizedGateway(msg.sender, sender), ERC7786RecipientInvalidGateway(msg.sender)); - require( - !_received[msg.sender].get(uint256(receiveId)), - ERC7786RecipientMessageAlreadyProcessed(msg.sender, receiveId) - ); + if (!_isAuthorizedGateway(msg.sender, sender)) { + revert ERC7786RecipientInvalidGateway(msg.sender); + } + if (_received[msg.sender].get(uint256(receiveId))) { + revert ERC7786RecipientMessageAlreadyProcessed(msg.sender, receiveId); + }⛔ Skipped due to learnings
Learnt from: ernestognw PR: OpenZeppelin/openzeppelin-contracts#5904 File: contracts/crosschain/ERC7786Receiver.sol:28-31 Timestamp: 2025-08-28T15:48:27.477Z Learning: Since Solidity 0.8.26, the require() function supports a new overload require(bool, Error) that allows using custom errors as the second parameter instead of just string messages. This feature was initially available only in the via-ir pipeline in 0.8.26, but became available in the legacy pipeline starting from Solidity 0.8.27.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
contracts/crosschain/ERC7786Recipient.sol (5)
14-17: Tighten docs: pluralization and wording
- “One or multiple gateway” → “One or multiple gateways”.
- “recognised” → “recognized” (match OZ’s US English style).
- * * {_isAuthorizedGateway}, an internal getter used to verify whether an address is recognised by the contract as a - * valid ERC-7786 destination gateway. One or multiple gateway can be supported. Note that any malicious address for + * * {_isAuthorizedGateway}, an internal getter used to verify whether an address is recognized by the contract as a + * valid ERC-7786 destination gateway. One or multiple gateways can be supported. Note that any malicious address for
20-22: Fix typo in docs (“manning” → “meaning”)Small clarity tweak.
- * This contract implements replay protection, manning that if two messages are received from the same gateway with the + * This contract implements replay protection, meaning that if two messages are received from the same gateway with the
53-55: Param name nit: prefer “gateway” over “instance”Align naming with call site and error types for consistency.
- function _isAuthorizedGateway(address instance, bytes calldata sender) internal view virtual returns (bool); + function _isAuthorizedGateway(address gateway, bytes calldata sender) internal view virtual returns (bool);
56-62: Consider dropping the gateway param from _processMessage (derive from msg.sender)Saves one stack slot and reduces parameter surface. Base can rely on msg.sender within the same context.
- _processMessage(msg.sender, receiveId, sender, payload); + _processMessage(receiveId, sender, payload);- function _processMessage( - address gateway, - bytes32 receiveId, - bytes calldata sender, - bytes calldata payload - ) internal virtual; + function _processMessage( + bytes32 receiveId, + bytes calldata sender, + bytes calldata payload + ) internal virtual;If you prefer explicitness for testing/auditing, keeping the param is fine—just confirm the tradeoff you want.
Also applies to: 48-48
26-27: Optional: expose a read-only “processed” queryHelps off-chain tooling/tests introspect replay state without duplicating BitMaps logic.
mapping(address gateway => BitMaps.BitMap) private _received; + + /// @notice Returns true if the (gateway, receiveId) message was already processed. + function processed(address gateway, bytes32 receiveId) public view returns (bool) { + return _received[gateway].get(uint256(receiveId)); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
contracts/crosschain/ERC7786Recipient.sol(1 hunks)test/crosschain/ERC7786Recipient.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/crosschain/ERC7786Recipient.test.js
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-08-29T13:17:07.068Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:22-31
Timestamp: 2025-08-29T13:17:07.068Z
Learning: In OpenZeppelin contracts, virtual functions that implement interfaces should use `public` visibility instead of `external`, even for calldata-heavy functions. This is because external functions cannot be called with `super` in derived contracts, which would limit extensibility. OpenZeppelin has strong guidelines to keep all public-facing virtual functions as `public` to maintain inheritance flexibility.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-09-04T09:13:21.278Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses yargs to configure the Solidity compiler version dynamically via command line arguments, with a default of '0.8.27' set on line 21 and the solidity configuration using argv.compile around lines 76-87, rather than hardcoded version strings.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-09-04T09:13:21.278Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses a sophisticated yargs-based configuration where the Solidity compiler version is set via argv.compiler (line 77) with a default of '0.8.27' defined in the yargs options (line 21), allowing flexible command-line overrides while maintaining a consistent default.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: coverage
- GitHub Check: slither
- GitHub Check: tests-foundry
- GitHub Check: tests
- GitHub Check: tests-upgradeable
- GitHub Check: halmos
🔇 Additional comments (1)
contracts/crosschain/ERC7786Recipient.sol (1)
42-47: Replay protection placement LGTMSetting the bit before calling user logic prevents double-exec even under reentrancy; revert in _processMessage reverts the bit set. Good placement.
…openzeppelin-contracts into crosschain/erc7786receiver
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
contracts/crosschain/ERC7786Recipient.sol (2)
56-62: Optional API simplification: drop redundant gateway paramSince
msg.senderis the gateway, consider removinggatewayfrom_processMessageto reduce stack usage; callers can readmsg.senderdirectly. Only if this aligns with intended flexibility.- function _processMessage( - address gateway, - bytes32 receiveId, - bytes calldata sender, - bytes calldata payload - ) internal virtual; + function _processMessage( + bytes32 receiveId, + bytes calldata sender, + bytes calldata payload + ) internal virtual;
31-36: Fix missing override and use public to preserve super() chainingImplementing an interface requires
override. Usepublicand keep itvirtualper OZ extensibility guidelines.Apply:
- ) external payable returns (bytes4) { + ) public payable virtual override returns (bytes4) {
🧹 Nitpick comments (3)
contracts/crosschain/ERC7786Recipient.sol (3)
14-17: Natspec grammar tweaksMinor clarity/US spelling.
- * * {_isAuthorizedGateway}, an internal getter used to verify whether an address is recognised by the contract as a - * valid ERC-7786 destination gateway. One or multiple gateway can be supported. Note that any malicious address for + * * {_isAuthorizedGateway}, an internal check used to verify whether an address is recognized by the contract as a + * valid ERC-7786 destination gateway. One or multiple gateways can be supported. Note that any malicious address for
42-47: Micro-gas: cache BitMap storage pointerAvoids two mapping reads; tiny win, clearer intent.
- // Prevent duplicate execution - if (_received[msg.sender].get(uint256(receiveId))) { - revert ERC7786RecipientMessageAlreadyProcessed(msg.sender, receiveId); - } - _received[msg.sender].set(uint256(receiveId)); + // Prevent duplicate execution + BitMaps.BitMap storage seen = _received[msg.sender]; + if (seen.get(uint256(receiveId))) { + revert ERC7786RecipientMessageAlreadyProcessed(msg.sender, receiveId); + } + seen.set(uint256(receiveId));
53-55: Document expected authorization patternsBriefly note common uses (e.g., chain-scoped sender restrictions per gateway) to guide implementers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/crosschain/ERC7786Recipient.sol(1 hunks)contracts/crosschain/README.adoc(1 hunks)contracts/interfaces/README.adoc(2 hunks)contracts/mocks/crosschain/ERC7786RecipientMock.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/mocks/crosschain/ERC7786RecipientMock.sol
- contracts/crosschain/README.adoc
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-08-29T13:17:07.068Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:22-31
Timestamp: 2025-08-29T13:17:07.068Z
Learning: In OpenZeppelin contracts, virtual functions that implement interfaces should use `public` visibility instead of `external`, even for calldata-heavy functions. This is because external functions cannot be called with `super` in derived contracts, which would limit extensibility. OpenZeppelin has strong guidelines to keep all public-facing virtual functions as `public` to maintain inheritance flexibility.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-09-04T09:13:21.278Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses yargs to configure the Solidity compiler version dynamically via command line arguments, with a default of '0.8.27' set on line 21 and the solidity configuration using argv.compile around lines 76-87, rather than hardcoded version strings.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
📚 Learning: 2025-09-04T09:13:21.278Z
Learnt from: Amxx
PR: OpenZeppelin/openzeppelin-contracts#5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses a sophisticated yargs-based configuration where the Solidity compiler version is set via argv.compiler (line 77) with a default of '0.8.27' defined in the yargs options (line 21), allowing flexible command-line overrides while maintaining a consistent default.
Applied to files:
contracts/crosschain/ERC7786Recipient.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: halmos
- GitHub Check: coverage
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: tests
🔇 Additional comments (2)
contracts/interfaces/README.adoc (2)
49-50: LGTM: ERC-7786 interfaces correctly added to the listEntries for IERC7786GatewaySource and IERC7786Recipient look consistent with ordering and naming elsewhere.
107-110: LGTM: ABI macros addedMacros for IERC7786GatewaySource and IERC7786Recipient follow the existing pattern.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
Fixes #5899
PR Checklist
npx changeset add)Summary by Sourcery
Migrate ERC7786Receiver from community by introducing an abstract receiver contract, accompanying mocks, and unit tests, and update the gateway interface parameter name
New Features:
Enhancements:
Tests:
Chores:
Summary by CodeRabbit
New Features
Documentation
Tests
Chores